Remove FixedShapedIterator
#131
Conversation
| Specialization of `BottomUpState`'s `BottomUpData` type. Contains the following: | ||
| * `nested_rulenode_iterator::NestedRulenodeIterator`: Iterator generating `RuleNodes` using a grammar rule to combine existing `RuleNodes` from the `bank`. | ||
| * `new_programs::Vector{RuleNode}`: Programs generated at the current depth level. Will be appended to the `bank` when starting a new depth level. | ||
| * `rules::Queue{Int}`: Grammar rules left to be used at the current depth level. |
There was a problem hiding this comment.
Can we maybe call this unused_rules?
|
|
||
| Returns the non-terminal rules in the order in which they appear in the grammar. | ||
| """ | ||
| function order( |
There was a problem hiding this comment.
order is too vague. I suggest we make the function name more descriptive (ordered_nonterminals?). Also, I believe this should be possible with a one-liner Queue(grammar.rules[grammar.isterminal]) or something like this.
| Returns the next program in `BUDepthIterator`'s iteration. Performs the following steps: | ||
| 1. Check whether `data.nested_rulenode_iterator` contains any program. | ||
| 2. Otherwise, pick the next rule from `data.rules` and generate all combinations of `RuleNodes` from the bank that could be combined using this rule. | ||
| 3. If `data.rules` is empty, call `_increase_depth!` and go to step 2. |
There was a problem hiding this comment.
I would prefer the docstring to not contain the steps of the function like this with all of the implementation details—that's what the source is for. Maybe we move the comments into inline comments and keep this part more general. I just worry the two will get out of sync of there are changes.
| Performs the following steps required when the depth is increased: | ||
| * Increase `data.depth`. | ||
| * Create a new `data.rules` queue consisting of the indices of non-terminal rules in the grammar. | ||
| * Copy all programs from `data.new_programs` to the `bank`. | ||
| * Go to Step 2. | ||
| """ |
There was a problem hiding this comment.
Same as above, this feels a bit verbose for a docstring.
| """ | ||
| init_bank(iter::BUDepthIterator)::BUDepthBank | ||
|
|
||
| Returns an initialized object of type `BUDepthBank`. For each symbol in the grammar (i.e. key in the dictionary), an empty `Vector{RuleNode}` (i.e. value in the dictionary) is allocated. | ||
| """ | ||
| function init_bank( | ||
| iter::BUDepthIterator | ||
| )::BUDepthBank |
There was a problem hiding this comment.
Do we have a motivation for not making this a constructor of BUDepthBank?
| """ | ||
| init_data(iter::BUDepthIterator)::BUDepthData | ||
|
|
||
| Returns an initialized object of type `BUDepthData`. The initialization consists of the following: | ||
| * `nested_rulenode_iterator` is set to an empty `NestedRulenodeIterator`. | ||
| * `rules` contains the indeces of terminal rules in the grammar. | ||
| * `new_programs` is an empty `Vector{RuleNode}`. | ||
| * `depth` is set to 1. | ||
| """ | ||
| function init_data( | ||
| iter::BUDepthIterator | ||
| )::BUDepthData |
There was a problem hiding this comment.
Same here—I think this can be a constructor.
| Observational equivalence (i.e. removing programs which yield the same outputs on all examples) is an optional optimization which could be used by `BottomUpIterator`s. | ||
| To enable it, add the following fields to the `BottomUpIterator` implementation: | ||
| - `problem::Union{Nothing, Problem{Vector{IOExample}}}. | ||
| - `obs_equivalence::Bool` (and set this to `true` when creating the `BottomUpIterator`). |
There was a problem hiding this comment.
I think this would ideally be a HasObsEquivalence trait instead, but if this approach works, I'm fine leaving it as is. It just makes the code for _has_obs_equivalence not-so-pretty.
|
|
||
| Returns an initialized object of type `BUDepthBank`. For each symbol in the grammar (i.e. key in the dictionary), an empty `Vector{RuleNode}` (i.e. value in the dictionary) is allocated. | ||
| """ | ||
| function init_bank( |
There was a problem hiding this comment.
| function init_bank( | |
| function BUUniformBank( |
possibly?
| * `new_programs` is an empty `Vector{RuleNode}`. | ||
| * `depth` is set to 1. | ||
| """ | ||
| function init_data( |
There was a problem hiding this comment.
| function init_data( | |
| function BUUniformData( |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #131 +/- ##
==========================================
+ Coverage 67.53% 71.26% +3.72%
==========================================
Files 21 20 -1
Lines 727 689 -38
==========================================
Hits 491 491
+ Misses 236 198 -38 ☔ View full report in Codecov by Sentry. |
|
Fixed failing CI due to exporting non-existent symbols. Nightly is still failing because changes from #128 are not on this branch yet. I would be in favor of rebasing this branch on top of |
Add `Aqua.jl` to test for this in the future as well as other Aqua tests. Ignoring the type piracies on `RuleNode`s and `AbstractGrammar`. Ideally, this functionality would be moved to `HerbCore`, but that can wait.
ReubenJ
force-pushed
the
remove_fixedshape_iterator
branch
from
2c16416 to
00fdacf
Compare
Addresses #130 . For iterating uniform trees, see
UniformIterator.